-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor/db #749
Refactor/db #749
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First round of feedback, but looks quite good already 👍
7452c73
to
eb15c7f
Compare
Well, I think this is good for testing, testing, testing... |
Note to myself:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did not see everything yet, but see some first comments. Generally, I am lacking a birds view, yet, need to get more familiar with the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second part of my review, left a couple of comments.
ba4de71
to
91feee4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few comments
I tried an upgrade with a handful of tables. The upgrade itself seemed to be successful, but on the web, nothing is show apart of two error messages. These were logged: Undefined array key -2 at /srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php#272{
"reqId": "APMX0Z3SEmTLfDXsiAEl",
"level": 3,
"time": "2024-01-17T10:31:50+00:00",
"remoteAddr": "127.0.0.1",
"user": "master",
"app": "PHP",
"method": "GET",
"url": "/master/index.php/apps/tables/table",
"message": "Undefined array key -2 at /srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php#272",
"userAgent": "Mozilla/5.0 (X11; Linux x86_64; rv:121.0) Gecko/20100101 Firefox/121.0",
"version": "29.0.0.3",
"data": {
"app": "PHP"
}
} OCA\\Tables\\Db\\Row2Mapper::getFilterExpression(): Argument #2 ($column) must be of type OCA\\Tables\\Db\\Column, null given, called in /srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php on line 272 in file '/srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php' line 282{
"reqId": "APMX0Z3SEmTLfDXsiAEl",
"level": 3,
"time": "2024-01-17T10:31:50+00:00",
"remoteAddr": "127.0.0.1",
"user": "master",
"app": "index",
"method": "GET",
"url": "/master/index.php/apps/tables/table",
"message": "OCA\\Tables\\Db\\Row2Mapper::getFilterExpression(): Argument #2 ($column) must be of type OCA\\Tables\\Db\\Column, null given, called in /srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php on line 272 in file '/srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php' line 282",
"userAgent": "Mozilla/5.0 (X11; Linux x86_64; rv:121.0) Gecko/20100101 Firefox/121.0",
"version": "29.0.0.3",
"exception": {
"Exception": "Exception",
"Message": "OCA\\Tables\\Db\\Row2Mapper::getFilterExpression(): Argument #2 ($column) must be of type OCA\\Tables\\Db\\Column, null given, called in /srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php on line 272 in file '/srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php' line 282",
"Code": 0,
"Trace": [
{
"file": "/srv/http/nextcloud/master/lib/private/AppFramework/App.php",
"line": 184,
"function": "dispatch",
"class": "OC\\AppFramework\\Http\\Dispatcher",
"type": "->",
"args": [
[
"OCA\\Tables\\Controller\\TableController"
],
"index"
]
},
{
"file": "/srv/http/nextcloud/master/lib/private/Route/Router.php",
"line": 315,
"function": "main",
"class": "OC\\AppFramework\\App",
"type": "::",
"args": [
"OCA\\Tables\\Controller\\TableController",
"index",
[
"OC\\AppFramework\\DependencyInjection\\DIContainer"
],
[
"tables.table.index"
]
]
},
{
"file": "/srv/http/nextcloud/master/lib/base.php",
"line": 1069,
"function": "match",
"class": "OC\\Route\\Router",
"type": "->",
"args": [
"/apps/tables/table"
]
},
{
"file": "/srv/http/nextcloud/master/index.php",
"line": 39,
"function": "handleRequest",
"class": "OC",
"type": "::",
"args": []
}
],
"File": "/srv/http/nextcloud/master/lib/private/AppFramework/Http/Dispatcher.php",
"Line": 170,
"Previous": {
"Exception": "TypeError",
"Message": "OCA\\Tables\\Db\\Row2Mapper::getFilterExpression(): Argument #2 ($column) must be of type OCA\\Tables\\Db\\Column, null given, called in /srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php on line 272",
"Code": 0,
"Trace": [
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php",
"line": 272,
"function": "getFilterExpression",
"class": "OCA\\Tables\\Db\\Row2Mapper",
"type": "->",
"args": [
[
"OC\\DB\\QueryBuilder\\QueryBuilder"
],
null,
"is-equal",
"Prime Admin XXVIII"
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php",
"line": 258,
"function": "getFilter",
"class": "OCA\\Tables\\Db\\Row2Mapper",
"type": "->",
"args": [
[
"OC\\DB\\QueryBuilder\\QueryBuilder"
],
[
[
-2,
"is-equal",
"Prime Admin XXVIII"
],
[
40,
"is-empty",
""
]
]
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php",
"line": 236,
"function": "getFilterGroups",
"class": "OCA\\Tables\\Db\\Row2Mapper",
"type": "->",
"args": [
[
"OC\\DB\\QueryBuilder\\QueryBuilder"
],
[
[
[
-2,
"is-equal",
"Prime Admin XXVIII"
],
[
40,
"is-empty",
""
]
]
]
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php",
"line": 139,
"function": "addFilterToQuery",
"class": "OCA\\Tables\\Db\\Row2Mapper",
"type": "->",
"args": [
[
"OC\\DB\\QueryBuilder\\QueryBuilder"
],
[
[
[
-2,
"is-equal",
"Prime Admin XXVIII"
],
[
40,
"is-empty",
""
]
]
],
"master"
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php",
"line": 674,
"function": "getWantedRowIds",
"class": "OCA\\Tables\\Db\\Row2Mapper",
"type": "->",
"args": [
"master",
6,
[
[
[
-2,
"is-equal",
"@my-name"
],
[
40,
"is-empty",
""
]
]
]
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Service/RowService.php",
"line": 516,
"function": "countRowsForView",
"class": "OCA\\Tables\\Db\\Row2Mapper",
"type": "->",
"args": [
[
"OCA\\Tables\\Db\\View",
1
],
"master",
[
[
"OCA\\Tables\\Db\\Column",
35
],
[
"OCA\\Tables\\Db\\Column",
36
],
[
"OCA\\Tables\\Db\\Column",
37
],
[
"OCA\\Tables\\Db\\Column",
38
],
[
"OCA\\Tables\\Db\\Column",
39
]
]
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Service/ViewService.php",
"line": 367,
"function": "getViewRowsCount",
"class": "OCA\\Tables\\Service\\RowService",
"type": "->",
"args": [
[
"OCA\\Tables\\Db\\View",
1
],
"master"
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Service/ViewService.php",
"line": 76,
"function": "enhanceView",
"class": "OCA\\Tables\\Service\\ViewService",
"type": "->",
"args": [
[
"OCA\\Tables\\Db\\View",
1
],
"master"
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Service/TableService.php",
"line": 198,
"function": "findAll",
"class": "OCA\\Tables\\Service\\ViewService",
"type": "->",
"args": [
[
"OCA\\Tables\\Db\\Table",
6
]
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Service/TableService.php",
"line": 124,
"function": "enhanceTable",
"class": "OCA\\Tables\\Service\\TableService",
"type": "->",
"args": [
[
"OCA\\Tables\\Db\\Table",
6
],
"master"
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Controller/TableController.php",
"line": 39,
"function": "findAll",
"class": "OCA\\Tables\\Service\\TableService",
"type": "->",
"args": [
"master"
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Controller/Errors.php",
"line": 16,
"function": "OCA\\Tables\\Controller\\{closure}",
"class": "OCA\\Tables\\Controller\\TableController",
"type": "->",
"args": [
"*** sensitive parameters replaced ***"
]
},
{
"file": "/srv/http/nextcloud/master/apps-repos/tables/lib/Controller/TableController.php",
"line": 38,
"function": "handleError",
"class": "OCA\\Tables\\Controller\\TableController",
"type": "->",
"args": [
[
"Closure"
]
]
},
{
"file": "/srv/http/nextcloud/master/lib/private/AppFramework/Http/Dispatcher.php",
"line": 232,
"function": "index",
"class": "OCA\\Tables\\Controller\\TableController",
"type": "->",
"args": []
},
{
"file": "/srv/http/nextcloud/master/lib/private/AppFramework/Http/Dispatcher.php",
"line": 138,
"function": "executeController",
"class": "OC\\AppFramework\\Http\\Dispatcher",
"type": "->",
"args": [
[
"OCA\\Tables\\Controller\\TableController"
],
"index"
]
},
{
"file": "/srv/http/nextcloud/master/lib/private/AppFramework/App.php",
"line": 184,
"function": "dispatch",
"class": "OC\\AppFramework\\Http\\Dispatcher",
"type": "->",
"args": [
[
"OCA\\Tables\\Controller\\TableController"
],
"index"
]
},
{
"file": "/srv/http/nextcloud/master/lib/private/Route/Router.php",
"line": 315,
"function": "main",
"class": "OC\\AppFramework\\App",
"type": "::",
"args": [
"OCA\\Tables\\Controller\\TableController",
"index",
[
"OC\\AppFramework\\DependencyInjection\\DIContainer"
],
[
"tables.table.index"
]
]
},
{
"file": "/srv/http/nextcloud/master/lib/base.php",
"line": 1069,
"function": "match",
"class": "OC\\Route\\Router",
"type": "->",
"args": [
"/apps/tables/table"
]
},
{
"file": "/srv/http/nextcloud/master/index.php",
"line": 39,
"function": "handleRequest",
"class": "OC",
"type": "::",
"args": []
}
],
"File": "/srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php",
"Line": 282
},
"message": "OCA\\Tables\\Db\\Row2Mapper::getFilterExpression(): Argument #2 ($column) must be of type OCA\\Tables\\Db\\Column, null given, called in /srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php on line 272 in file '/srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php' line 282",
"exception": {},
"CustomMessage": "OCA\\Tables\\Db\\Row2Mapper::getFilterExpression(): Argument #2 ($column) must be of type OCA\\Tables\\Db\\Column, null given, called in /srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php on line 272 in file '/srv/http/nextcloud/master/apps-repos/tables/lib/Db/Row2Mapper.php' line 282"
}
} |
Notes from the migration test call as separate comments so they can be marked as resolved:
Exception for the missing column for data
|
|
28e1a67
to
84e7f00
Compare
This comment was marked as resolved.
This comment was marked as resolved.
…olumns yet) Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
…lumns yet) Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Co-authored-by: Arthur Schiwon <blizzz@arthur-schiwon.de> Signed-off-by: Florian <dev@d10t.de>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
This reverts commit b87f19c.
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
- prepare LegacyRowMapper.php and Row2.php - add LegacyRowMapperTest.php Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Co-authored-by: Julius Härtl <jus@bitgrid.net> Signed-off-by: Florian <dev@d10t.de>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com> fixup! fix: replace placeholder for selection and check columns
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
… migration Signed-off-by: Julius Härtl <jus@bitgrid.net>
…cessible row is used Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
- add data-cy to form sections - add general commands to fill in data in forms for selection column - add test cases for all selection types - add basic combination test cases Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com> fix: test urls Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
452a837
to
55e9c29
Compare
closes #498
This should be tested carefully as it bring breaking changes with it.